fix(ActiveSelection): 🚀 multiple nested selection 🗂️ #7882

ShaMan123 · 2022-04-18T11:35:27Z

Multiple Nested Selection

This PR finalizes (so it seems) the group rewrite!

Gist

refactored _chooseObjectsToRender
made canvas hold a constant ref to ActiveSelection instead of creating new ones all time (important for devs wanting to attach events to it for customization, see fix(ActiveSelection): 🔃 preserveObjectStacking 📌 #7878)
introduced Group#invalidate - to handle children/ActiveSelection state changes that should propagate to group + an option for layout customization (progress - very experimental and unstable at the moment, remains optional)
invalidate owning group of object under ActiveSelection

The progess layout trigger is designed to enable updating group's size dynamically but it introduces unwanted changes and craziness. e.g. when object is rotating. We should consider if and how to support it. I think it should remain as is, the dev can override getLayoutStrategyResult and do what they want.

TODO

figure out what to do with exporters (toDataUrl, toCanvas) on group while some objects are selected and canvas.preserveObjectStacking = false and therefore not rendered by group - DONE 79db7d5
test/visual/golden/group-layout/selected_object.png - seems the selected rect is doubled in opacity...
rethink ActiveSelection caching - why not?

Dev

Video was heavy so visuals are poor but perf is amazing

closes #6776

test code to paste in the kitchensink demo

// clear canvas
canvas.clear();
 canvas.preserveObjectStacking = false;
    const circle = new fabric.Circle({ left: 100, top: 50, radius: 50 });
    const text = new fabric.Text('', { evented: false });
    const itext = new fabric.IText('Edit me\nfit-content layout', { left: 100, top: 150 });
    const g = new fabric.Group([
      new fabric.Rect({
        top: 200,
        width: 50,
        height: 50,
        fill: 'red',
        opacity: 0.3
      }),
      circle,
      itext
    ], {
      backgroundColor: 'blue',
      subTargetCheck: true,
      interactive: true
      //objectCaching: false
    });
    canvas.add(
      text,
      g
    );
    g.clone().then(clone => {
      clone.item(2).set({ text: 'Edit me\nclip path layout' });
      clone.set({
        backgroundColor: 'magenta',
        clipPath: new fabric.Circle({
          radius: 110,
          group: clone
          //left: -50, top: -50,
          // scaleX:0.6
        }),
        layout: 'clip-path'
      });
     // canvas.add(clone);
    });
    g.clone().then(clone => {
      clone.item(2).set({ text: 'Edit me\nclip path layout' });
      clone.set({
        backgroundColor: 'yellow',
        clipPath: new fabric.Circle({
          radius: 110,
          //left: -50, top: -50,
          originX: 'center', originY: 'center',
          group: clone
        }),
        layout: 'clip-path'
      });
      canvas.add(clone);
      setInterval(() => {
        clone._set('dirty', true)
      }, 0);
    });
    g.clone().then(clone => {
      clone.item(2).set({ text: 'Edit me\nabsolute positioned\nclip path layout' });
      clone.set({
        backgroundColor: '#0dcaf0',
        clipPath: new fabric.Circle({
          radius: 110, originX: 'center', originY: 'center',
          absolutePositioned: true, left: 50, top: 150, skewX:20
        }),
        layout: 'clip-path',

      });
      canvas.insertAt(clone, 0);
    });
    canvas.on('after:render', () => {
      text.set('text', `circle is on screen? ${circle.isOnScreen()}`);
      text.dirty && canvas.requestRenderAll();
    });

#7744 (review)

in case object is a collection

github-actions · 2022-05-01T16:27:58Z

Code Coverage Summary

> fabric@5.1.0 coverage:report
> nyc report --reporter=lcov --reporter=text

-----------|---------|----------|---------|---------|-----------------------------------------------
File       | % Stmts | % Branch | % Funcs | % Lines | Uncovered Line #s                             
-----------|---------|----------|---------|---------|-----------------------------------------------
All files  |   83.02 |    76.01 |   85.49 |   82.74 |                                               
 fabric.js |   83.02 |    76.01 |   85.49 |   82.74 | ...,30892,30966,30977-31042,31165,31264,31500 
-----------|---------|----------|---------|---------|-----------------------------------------------

ShaMan123

updated from master

github-actions · 2022-05-02T10:36:41Z

Code Coverage Summary

> fabric@5.1.0 coverage:report
> nyc report --reporter=lcov --reporter=text

-----------|---------|----------|---------|---------|-----------------------------------------------
File       | % Stmts | % Branch | % Funcs | % Lines | Uncovered Line #s                             
-----------|---------|----------|---------|---------|-----------------------------------------------
All files  |   83.05 |    76.06 |   85.49 |   82.77 |                                               
 fabric.js |   83.05 |    76.06 |   85.49 |   82.77 | ...,30901,30975,30986-31051,31174,31273,31509 
-----------|---------|----------|---------|---------|-----------------------------------------------

stale · 2022-06-12T12:56:02Z

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

github-actions · 2022-06-13T06:31:26Z

Code Coverage Summary

> fabric@5.1.0 coverage:report
> nyc report --reporter=lcov --reporter=text

-----------|---------|----------|---------|---------|-----------------------------------------------
File       | % Stmts | % Branch | % Funcs | % Lines | Uncovered Line #s                             
-----------|---------|----------|---------|---------|-----------------------------------------------
All files  |   83.07 |    76.09 |   85.55 |   82.79 |                                               
 fabric.js |   83.07 |    76.09 |   85.55 |   82.79 | ...,30911,30985,30996-31061,31184,31283,31519 
-----------|---------|----------|---------|---------|-----------------------------------------------

stale · 2022-06-28T00:17:07Z

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

ShaMan123

preservevObjectStacking plays a role in this PR in group rendering logic.
We can work around it, we need to think how though

ShaMan123 · 2022-06-30T14:41:42Z

src/canvas.class.js

+      if (!this.preserveObjectStacking && activeObject) {
+        return this._objects.filter(function (object) {
+          return !object.group && object !== activeObject;
+        }).concat(activeObject);


We can stop this foolishness by doing what #8034 does on Group
I didn't touch ActiveSelection because of this piece of logic but I don't see a reason why not if we want to drop this ugly part.

ShaMan123 · 2022-06-30T14:42:30Z

src/shapes/active_selection.class.js

+        this.forEachObject(function (object) {
+          var group = object.__owningGroup;
+          if (group && invalidatedGroups.indexOf(group) === -1) {
+            group.invalidate(invalidationContext);


maybe I want to add a check for objects that are group de-facto but aren't really groups:

Suggested change

group.invalidate(invalidationContext);

group.invalidate && group.invalidate(invalidationContext);

stale · 2022-07-30T19:18:23Z

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

This allows objects on active selection to enter I want to change that related #7882

ShaMan123 · 2022-09-14T19:55:03Z

This PR will wait until caching is decided #7874

ShaMan123

files to port to #8665 :

src/shapes/active_selection.class.js
src/shapes/group.class.js
src/shapes/object.class.js
test/unit/activeselection.js
test/unit/util.js
test/visual/group_layout.js - not sure what the test is

ShaMan123 · 2023-02-06T15:47:03Z

src/shapes/active_selection.class.js

+     * @param {string} key
+     * @param {*} value
+     */
+    _set: function (key, value) {


this is the only part of active selection that hasn't been ported to #8665

ShaMan123 · 2023-02-06T22:45:04Z

what remains to port from this PR is caching logic and maybe a bit of rendering

ShaMan123 · 2023-02-06T22:49:01Z

src/shapes/group.class.js

+      var activeObject = this.canvas && this.canvas.getActiveObject && this.canvas.getActiveObject();
+      // if we are adding the activeObject in a group
+      if (activeObject && (activeObject === object || object.isDescendantOf(activeObject))) {
+        this._activeObjects.push(object);
+      }


this part....\

asturur · 2024-01-14T16:47:05Z

capturing this test case here since the functionality is not working with the current layout manager and adding an issue to describe it. The code here is not reconciliable

ShaMan123 · 2024-01-14T17:24:27Z

Reviewing the code seems we can test to see if it works because I don't see too much here that isn't in the codebase

ShaMan123 added 30 commits April 3, 2022 23:32

apply 7744 patch (no binaries)

1a191a7

apply 7744 patch (binaries)

26e8392

commit #7744 review

7d793b3

#7744 (review)

patch2

2708029

Update spray_brush.class.js

02a8ffa

enable text editing under group

0d3690f

migrate back to isDescendantOf

ff0346a

render bg

812f02a

fix(): clear canvas from entire object tree

053946c

in case object is a collection

fix(): center object in case object is nested

db76c44

Update eraser_brush.mixin.js

04aad5b

Update canvas.class.js

8c1c535

Update canvas_events.mixin.js

3fd4af2

lint

58e9be2

safeguard from adding group to itself

c2ef2f5

Update object_ancestry.mixin.js

f0145b5

patch tests from v6!

cb20b81

Update canvas_grouping.mixin.js

347f80d

Update canvas.class.js

a56d55e

Update group_layout.js

2a426e7

Update group_layout.js

5e3fc1f

remove redundant logic

2b1f680

Update canvas_grouping.mixin.js

1ad5cc8

fix(): calling sendPointToPlane

11c828f

Update group.class.js

b6fa3dc

refactor enterGroup

e9afc89

invalidate groups after active selection change

d8f8251

_shouldSetNestedCoords

c58a274

perf(): refrain from contains

c0f5571

Update canvas_events.mixin.js

9a223d0

fabricjs deleted a comment from github-actions bot Apr 18, 2022

ShaMan123 mentioned this pull request Apr 20, 2022

fix(V6): nested selection 🔧 fix canvas exporting 🧩 #7893

Closed

3 tasks

Merge branch 'master' into v6-nested-selection

864f174

ShaMan123 commented May 1, 2022

View reviewed changes

Merge branch 'master' into v6-nested-selection

8c317fe

stale bot added the stale Issue marked as stale by the stale bot label Jun 12, 2022

ShaMan123 removed the stale Issue marked as stale by the stale bot label Jun 13, 2022

Merge branch 'master' into v6-nested-selection

91a7f53

stale bot added the stale Issue marked as stale by the stale bot label Jun 28, 2022

ShaMan123 removed the stale Issue marked as stale by the stale bot label Jun 28, 2022

ShaMan123 commented Jun 30, 2022

View reviewed changes

stale bot added the stale Issue marked as stale by the stale bot label Jul 30, 2022

ShaMan123 removed the stale Issue marked as stale by the stale bot label Aug 1, 2022

ShaMan123 added a commit that referenced this pull request Aug 9, 2022

fix(): filter objects before entering canvas

ac2be16

This allows objects on active selection to enter I want to change that related #7882

ShaMan123 mentioned this pull request Feb 6, 2023

fix(): selection logic to support nested multiselection #8665

Merged

1 task

ShaMan123 commented Feb 6, 2023

View reviewed changes

ShaMan123 marked this pull request as draft February 19, 2023 06:08

asturur closed this Jan 14, 2024

asturur mentioned this pull request Jan 14, 2024

ci(): Add a test case from the multiple selection use case for groups #9599

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix(ActiveSelection): 🚀 multiple nested selection 🗂️ #7882

fix(ActiveSelection): 🚀 multiple nested selection 🗂️ #7882

ShaMan123 commented Apr 18, 2022 •

edited

Loading

github-actions bot commented May 1, 2022 •

edited

Loading

ShaMan123 left a comment

github-actions bot commented May 2, 2022 •

edited

Loading

stale bot commented Jun 12, 2022

github-actions bot commented Jun 13, 2022 •

edited

Loading

stale bot commented Jun 28, 2022

ShaMan123 left a comment

ShaMan123 Jun 30, 2022

ShaMan123 Jun 30, 2022

stale bot commented Jul 30, 2022

ShaMan123 commented Sep 14, 2022

ShaMan123 left a comment

ShaMan123 Feb 6, 2023

ShaMan123 commented Feb 6, 2023

ShaMan123 Feb 6, 2023

asturur commented Jan 14, 2024

ShaMan123 commented Jan 14, 2024

	group.invalidate(invalidationContext);
	group.invalidate && group.invalidate(invalidationContext);

fix(ActiveSelection): 🚀 multiple nested selection 🗂️ #7882

fix(ActiveSelection): 🚀 multiple nested selection 🗂️ #7882

Conversation

ShaMan123 commented Apr 18, 2022 • edited Loading

Multiple Nested Selection

Gist

TODO

Dev

github-actions bot commented May 1, 2022 • edited Loading

Code Coverage Summary

ShaMan123 left a comment

Choose a reason for hiding this comment

github-actions bot commented May 2, 2022 • edited Loading

Code Coverage Summary

stale bot commented Jun 12, 2022

github-actions bot commented Jun 13, 2022 • edited Loading

Code Coverage Summary

stale bot commented Jun 28, 2022

ShaMan123 left a comment

Choose a reason for hiding this comment

ShaMan123 Jun 30, 2022

Choose a reason for hiding this comment

ShaMan123 Jun 30, 2022

Choose a reason for hiding this comment

stale bot commented Jul 30, 2022

ShaMan123 commented Sep 14, 2022

ShaMan123 left a comment

Choose a reason for hiding this comment

ShaMan123 Feb 6, 2023

Choose a reason for hiding this comment

ShaMan123 commented Feb 6, 2023

ShaMan123 Feb 6, 2023

Choose a reason for hiding this comment

asturur commented Jan 14, 2024

ShaMan123 commented Jan 14, 2024

ShaMan123 commented Apr 18, 2022 •

edited

Loading

github-actions bot commented May 1, 2022 •

edited

Loading

github-actions bot commented May 2, 2022 •

edited

Loading

github-actions bot commented Jun 13, 2022 •

edited

Loading